Clean up legacy styles and remove bs3-compat.scss #4267
Conversation
|
@sturkel89 I hope you could check this set of changes. This is the final big change to eliminate legacy styles (I hope!). Sorry for it being so large, but thought it would be better to keep all these together because many of changes touch same areas in the UI. |
There was a problem hiding this comment.
I tested this PR in both themes and in the mobile emulator, comparing test and dev. (I only tested on Chrome.) I found a number of little quirks that should be pretty easy to fix.
Item record top menu: there's no down arrow in Export Record (both themes)
The down arrow is missing, but the functionality is still there; if you click the button, the three export options appear.
UPDATE: This is fixed!
More button in top Suggested Topics area looks weird (both themes)
On dev, the More button is rounded with a faint, clean outline.
On test, the More button is squared off, short, and dark outlining is present whether the button is clicked or not. Both the More and Less buttons in this feature have the same appearance.
UPDATE: This is fixed, but looks lighter on test than on dev.
Sidebar facet listings look weird (both themes)
On test, in bootstrap5, there's a gap between the header field and the list of filters, and the More button with the arrow looks weird (similar to the More button in Suggested Topics).
In sandal5, the gap is present, but the More button looks okay.
UPDATE: This is fixed!
Text Area boxes are three lines high on test but one line high on dev (both themes)
We think that this is an improvement and a reversion to how things used to be! So, this is more of a note than a request to change things.
Example: Comment box on Bootstrap5
Example: Create a List on mobile, Sandal5
(NB: there are two other tiny issues with this screen: inconsistent punctuation in the labels for the two boxes, and something is weird with spacing of the breadcrumbs. I'll keep track of those in a different document.)
UPDATE: This is fixed!
Record tags: alignment/spacing is different between test and dev
In both themes, there's more horizontal padding inside tag buttons. I don't have a strong opinion on whether this is an improvement or not.
In Sandal5, the padding is the main difference in terms of tag appearance and behavior; wrapping behavior is pretty similar between test and dev.
In Bootstrap5, wrapping behavior is pretty different between test and dev.
Bootstrap5: in test, three short tags need a very wide browser to display all in one row. This is the minimal width for having all three on one line; any narrower, and they wrap to two rows.
In dev, this is the width that's needed to display them all in one row:
Here's the mobile view of the same three tags in bootstrap5:
UPDATE: This is fixed!
There was a problem hiding this comment.
Looks like the merge of #4238 has introduced some conflicts. Sorry about that!
|
@demiankatz No problem, that was to be expected! |
…lly-hidden-focusable.
- form-select is now alone without form-control - Some margins adjusted and feedback admin form cleaned up
…with float-start.
EreMaijala
force-pushed
the
dev-bs5-scss-cleanup
branch
from
1e07199 to
338ca0f
Compare
Now unified with toolbar-btn which was also tweaked a bit with a border and improved contrast.
|
@sturkel89 Thanks for the comments! All should be fixed now. Remarks:
|
There was a problem hiding this comment.
I see that all of my requested changes have been made! Looks great.
In terms of the More/Less buttons in Suggested Topics within your search, I see what you're saying. If the intention is to make these More/Less buttons slightly de-emphasized, you've been successful. As long as it's not an accessibility violation to have such a subtle-looking button, I'm okay with it.
Test:
Dev:
|
@sturkel89 I verified that the contrast is high enough, and actually the toolbar buttons now have better contrast between text and background than before, because I made the text a notch darker. |
There was a problem hiding this comment.
All looks reasonable to me. Thanks, @EreMaijala and @sturkel89!
demiankatz
added
the
architecture
|
Regarding the changelog entry: there is already a link from the changelog to the bootstrap5 wiki page. I added a link from that page to this PR for reference (since there was already a link to the PR removing the JS layer, and this complements that). @EreMaijala, do you think any further documentation work is needed? |
This is collection of sort-of independent commits with the aim to remove the Bootstrap 3 compatibility layer.
Notable changes:
btn btn-defaultcontinues to work.form-controlandform-selectclasses now only haveform-select.sr-onlyis nowvisually-hiddensr-only-focusableis nowvisually-hidden-focusablecontrol-labelis nowform-labelpull-rightis nowfloat-end(flipno longer needed)pull-leftis nowfloat-start(flipno longer needed)$something-borderrenamed to$something-border-color.Things to check during testing:
TODO: